Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix suggestion of additional pub when using pub pub fn ... #87901

Merged
merged 6 commits into from
Dec 18, 2021
Merged

Fix suggestion of additional pub when using pub pub fn ... #87901

merged 6 commits into from
Dec 18, 2021

Conversation

poliorcetics
Copy link
Contributor

Fix #87694.

Marked as draft to start with because I want to explore doing the same fix for const const fn and other repeated-but-valid keywords.

@rustbot label A-diagnostics D-invalid-suggestion T-compiler

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 10, 2021
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2021
@bors
Copy link
Contributor

bors commented Aug 18, 2021

☔ The latest upstream changes (presumably #87781) made this pull request unmergeable. Please resolve the merge conflicts.

@poliorcetics poliorcetics marked this pull request as ready for review August 21, 2021 12:41
@poliorcetics
Copy link
Contributor Author

This is now ready for review, it suggests the correct fix for both visibility and keywords

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2021
@bors
Copy link
Contributor

bors commented Sep 16, 2021

☔ The latest upstream changes (presumably #88992) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

Ping from triage:
@poliorcetics can you please address the merge conflict?

@pnkfelix pnkfelix assigned pnkfelix and unassigned matthewjasper Oct 21, 2021
@pnkfelix
Copy link
Member

While we wait for the author to address the merge conflicts, I'll plan to review.

@poliorcetics
Copy link
Contributor Author

Very sorry for the long response time, I have been otherwise occupied these past months.

I'll try to rebase this over the weekend

@poliorcetics poliorcetics force-pushed the pub-pub-pub branch 2 times, most recently from d95a128 to e99e33d Compare October 24, 2021 13:11
@poliorcetics
Copy link
Contributor Author

Rebase done, integrated suggestion

@bors
Copy link
Contributor

bors commented Nov 7, 2021

☔ The latest upstream changes (presumably #90671) made this pull request unmergeable. Please resolve the merge conflicts.

@poliorcetics
Copy link
Contributor Author

Sorry about commit c9d1d0b making things harder to review, I had to satisfy tidy and choose to do it once for all following Prs that will add tests to this folder

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Dec 1, 2021

☔ The latest upstream changes (presumably #91418) made this pull request unmergeable. Please resolve the merge conflicts.

@camelid
Copy link
Member

camelid commented Dec 12, 2021

@poliorcetics why are you moving so many tests as part of this PR? If you move them, it should be as a separate PR.

@poliorcetics
Copy link
Contributor Author

@poliorcetics why are you moving so many tests as part of this PR? If you move them, it should be as a separate PR.

As said in the original commit, there were too many tests in the folder. This has been fixed in a commit in a separate PR in between so I removed the commit on mine.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jackh726
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 18, 2021

📌 Commit be33ca7 has been approved by jackh726

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 18, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 18, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2021
@bors
Auto merge of rust-lang#92059 - matthiaskrgr:rollup-bainfyh, r=matthi…
208ced6 
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#87901 (Fix suggestion of additional `pub` when using `pub pub fn ...`)
 - rust-lang#89090 (Lint bare traits in AstConv.)
 - rust-lang#91818 (Show the unused type for `unused_results` lint)
 - rust-lang#91910 (miri: lift restriction on extern types being the only field in a struct)
 - rust-lang#91928 (Constify (most) `Option` methods)
 - rust-lang#91975 (Move generator check earlier in inlining.)
 - rust-lang#92016 (builtin_macros: allow external consumers for AsmArgs parsing)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 54e7946 into rust-lang:master Dec 18, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank left review comments

@pnkfelix pnkfelix pnkfelix left review comments

Assignees

@pnkfelix pnkfelix

Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.

Error suggests additional pub when using pub pub fn ...